fix(@angular/ssr): validate host headers to prevent header-based SSRF#32516
fix(@angular/ssr): validate host headers to prevent header-based SSRF#32516alan-agius4 wants to merge 2 commits intoangular:mainfrom
Conversation
a44aef7 to
8778533
Compare
757306a to
de79ce3
Compare
faf18cf to
aa6ee92
Compare
dgp1130
left a comment
There was a problem hiding this comment.
Left some general suggestions, but nothing major. Feel free to ignore or come back to any of these later rather than treating them as blocking this PR. The two main things to confirm are really just:
- Should we commit to
${HOSTNAME}/${NG_ALLOWED_HOSTS}right now, or come back later? - Should we error when
allowedHostsis configured and we receive a request for a disallowed host, to be up front about a misconfiguration rather than silently deoptimizing?
| let document = opts.document; | ||
| if (!document && opts.documentFilePath) { | ||
| document = await this.getDocument(opts.documentFilePath); | ||
| } |
There was a problem hiding this comment.
Question: What's the purpose of reading the document here? Why do we need this?
There was a problem hiding this comment.
Document in this is case is the index.html which we need to serve it to render CSR.
| const envNgAllowedHosts = processEnv['NG_ALLOWED_HOSTS']; | ||
| if (envNgAllowedHosts) { | ||
| const hosts = envNgAllowedHosts.split(','); | ||
| for (const host of hosts) { | ||
| const hostTrimmed = host.trim(); | ||
| if (hostTrimmed) { | ||
| allowedHosts.add(hostTrimmed); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider: Do you want to tackle this now, as something we'll probably have to support for a few versions, or leave it off and require allowedHosts to be specified in the build options in the short term until we come back to this in v22+?
I agree this is probably a good direction, but I'm wondering if it might benefit from more discussion, and I don't think we need to have it right now. Would it make sense to hold off before committing to a new public API here?
There was a problem hiding this comment.
My reason for this is that you'd be able to provide the allowedHost at runtime where using env variables is not an option. For instance in worker context.
If you feel we should hold this off, I am fine with this. Please let me know.
|
|
||
| const key = name.toLowerCase(); | ||
| if (HOST_HEADERS_TO_VALIDATE.has(key)) { | ||
| verifyHostAllowed(key, value, allowedHosts); |
There was a problem hiding this comment.
Observation: I could see this failing some apps who iterate through headers like:
for (const [name, value] of Object.entries(req.headers)) {
if (!name.startsWith('x-myapp-*')) continue;
doSomethingWith(name, value);
}In this case, they're not really using host or x-forwarded-host, but they will read the values and probably trigger the CSR deopt. I don't think there's anything we can meaningfully do about this, but pointing out the case of iterating through all the headers which isn't too uncommon.
There was a problem hiding this comment.
They will actually not trigger the CSR deopt. in this case they will not be validated.
| } | ||
| } | ||
|
|
||
| const xForwardedPort = getFirstHeaderValue(headers.get('x-forwarded-port')); |
There was a problem hiding this comment.
Quick question: do we normalize headers and lowercase the names earlier in the code?
There was a problem hiding this comment.
No we do not normalize headers names, the headers.get() is case insensitive.
This change introduces strict validation for `Host`, `X-Forwarded-Host`, `X-Forwarded-Proto`, and `X-Forwarded-Port` headers in the Angular SSR request handling pipeline, including `CommonEngine` and `AngularAppEngine`.
Previously, the application engine constructed the base URL for server-side rendering using these headers without validation. This could allow an attacker to manipulate the headers to steer relative `HttpClient` requests to arbitrary internal or external hosts (SSRF).
The validation rules are:
- `Host` and `X-Forwarded-Host` headers are validated against a strict allowlist.
- `Host` and `X-Forwarded-Host` headers cannot contain path separators.
- `X-Forwarded-Port` header must be numeric.
- `X-Forwarded-Proto` header must be `http` or `https`.
Requests with invalid or disallowed headers will now log an error and fallback to Client-Side Rendering (CSR). In a future major version, these requests will be rejected with a 400 Bad Request.
Note: Most cloud providers and CDNs already validate these headers before the request reaches the text application, but this change adds an essential layer of defense-in-depth.
**AngularAppEngine Users:**
To exclude safe hosts from validation, configure the `allowedHosts` option in `angular.json` to include all domain names where your application is deployed.
Example configuration in `angular.json`:
```json
"architect": {
"build": {
"options": {
"security": {
"allowedHosts": ["example.com", "*.trusted-example.com"]
}
}
}
}
```
or
```ts
const appEngine = new AngularAppEngine({
allowedHosts: ["example.com", "*.trusted-example.com"]
})
const appEngine = new AngularNodeAppEngine({
allowedHosts: ["example.com", "*.trusted-example.com"]
})
```
**CommonEngine Users:**
If you are using `CommonEngine`, you must now provide the `allowedHosts` option when initializing or rendering your application.
Example:
```typescript
const commonEngine = new CommonEngine({
allowedHosts: ["example.com", "*.trusted-example.com"]
});
```
The application also respects `NG_ALLOWED_HOSTS` (comma-separated list) environment variable for authorizing hosts.
da7a6aa to
301a633
Compare
9feb3c7 to
b0b1a97
Compare
I removed
Updated the logic to do this. |
50ff773 to
fe3fbba
Compare
fe3fbba to
e38778a
Compare
e38778a to
a9a660f
Compare
This change introduces strict validation for
Host,X-Forwarded-Host,X-Forwarded-Proto, andX-Forwarded-Portheaders in the Angular SSR request handling pipeline, includingCommonEngineandAngularAppEngine.Previously, the application engine constructed the base URL for server-side rendering using these headers without validation. This could allow an attacker to manipulate the headers to steer relative
HttpClientrequests to arbitrary internal or external hosts (SSRF).The validation rules are:
HostandX-Forwarded-Hostheaders are validated against an authorized allowlist.HostandX-Forwarded-Hostheaders cannot contain path separators.X-Forwarded-Portheader must be numeric.X-Forwarded-Protoheader must behttporhttps.Requests with invalid or disallowed headers will now log an error and fallback to Client-Side Rendering (CSR). In a future major version, these requests will be rejected with a 400 Bad Request.
Note: Most cloud providers and CDNs already validate these headers before the request reaches the text application, but this change adds an essential layer of defense-in-depth.
AngularAppEngine Users:
To exclude safe hosts from validation, configure the
allowedHostsoption inangular.jsonto include all domain names where your application is deployed.Example configuration in
angular.json:or
CommonEngine Users:
If you are using
CommonEngine, you must now provide theallowedHostsoption when initializing or rendering your application.Example:
The application also respects
NG_ALLOWED_HOSTS(comma-separated list) andHOSTNAMEenvironment variables for authorizing hosts.